🦋 Changeset detected

Latest commit: 754bed9

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Not sure whether to use a minor or patch for this one 🤔. Settled with minor for now.

This is a breaking change. It should be a major bump.

The documentation needs to be updated as well.
https://docs.slatejs.org/walkthroughs/06-saving-to-a-database

I'm curious whether this misses some subtlety in the rendering code that I'm unaware of. Please do educate me if so

onChange should not be the only way of changing the Component's value. If you rely on a database to render your content, you'll also want to pass the new value using the value prop. In that case you don't want to rely on a local state that is changed with onChange.

An example would be when you need to display someone else changes to the value.

This is a breaking change. It should be a major bump.

Pre 1.0 beta so not a major bump.

I'm curious whether this misses some subtlety in the rendering code that I'm unaware of. Please do educate me if so

onChange should not be the only way of changing the Component's value. If you rely on a database to render your content, you'll also want to pass the new value using the value prop. In that case you don't want to rely on a local state that is changed with onChange.

An example would be when you need to display someone else changes to the value.

How do you reconcile that with the discussion at #3575 (comment) ?

I am ok with either way. But the documentation over here https://docs.slatejs.org/walkthroughs/06-saving-to-a-database should be updated.

Besides the docs, the examples should probably also be updated to reflect this change which means just passing initialValue directly to the value prop instead of the value state that is currently kept in all examples.

It is not the right way for an api, you shouldn't changed it. better to have the same signature as the native input api

First of all, I want to say thanks to @ianstormtaylor and @bryanph for maintaining and continuing to build a great library.

Regarding these changes, I wonder whether they actually fix the issue mentioned in #3575: slate used to offer an API similar to any generic input, whereby it exposed props such as value and onChange. This enabled users to interact and maintain the value of the input with a useState hook, as below:

const Editor = () => {
  const [value, setValue] = useState('')
  return (
    <input value={value} onChange={(e) => setValue(e.target.value)} />
  )
}

Consequently, these changes entailed that to be able to interact with the value outside of slate the code would need to be changed from this:

const Editor = () => {
  const editor = useMemo(() => withReact(createEditor())), [])
  const [value, setValue] = useState('')
  return (
    <Slate
       editor={editor}
       value={value}
       onChange={setValue}
    >
      ...
    </Slate>
  )
}

to this:

const Editor = () => {
  const editor = useMemo(() => withReact(createEditor())), [])
  const [value, setValue] = useState('')
  editor.children = value
  return (
    <Slate
       editor={editor}
       value={value}
       onChange={setValue}
    >
      ...
    </Slate>
  )
}

Note the need to manually set the value of editor.children, which if I'm not mistaken can cause a Cannot resolve a DOM point from Slate point. error.

On a separate note, I think changes to the API should be added to the changelog as "Breaking Changes" instead of "Minor Changes" specially if a previous behaviour is no longer supported. I personally find it helpful to use standardised versioning system like Semantic Versioning and Keep a Changelog.

Thanks!

On a separate note, I think changes to the API should be added to the changelog as "Breaking Changes" instead of "Minor Changes" specially if a previous behaviour is no longer supported. I personally find it helpful to use standardised versioning system like Semantic Versioning and Keep a Changelog.

In general I agree, though we have not yet reached a 1.0 release. I would say at this point every minor update has likely broken someone somewhere. I'm not 100% certain what will lead us to decide that we're at 1.0, but I still see a lot of core things to improve and make more flexible before I would consider Slate stable.

@KarimNahas The problem is that whenever editor.children gets changed outside of slate's knowledge it has the potential to cause different inconsistencies to happen including the Cannot resolve a DOM point from Slate point error you were talking about or errors with undoing changes that no longer exist. Previously setting the value prop was simply an implicit way of just setting the editor.children value (see below)

I find people generally get confused by the "passing value" API as it seems to indicate that's how you propagate changes, the "reacty" way. But in reality slate operates on operations and keeps its internal state consistent using them. An API that reflects this behavior is therefore desirable was my reasoning.

Also see these two comments: #3575 (comment) and #3575 (comment)

That said I do understand the desire to reset the editor to some particular value. Perhaps we could provide a convenience method for this? Something like Editor.reset(editor)? That way we can guarantee it executes at the right time, avoiding nasty inconsistency problems.

@bryanph I definitely see how, since slate uses operations to mutate its state, using a controlled value pattern would be inconsistent. Specially considering that onChange is triggered for all operations including set_position.

Consequently it think exposing / creating a method that enables the user to safely mutate the editor children would definitely be nice. What about adding a method called resetNodes in NodeTransforms as follows:

export const NodeTransforms: NodeTransforms = {
  ...

  /**
   * resetNodes resets the value of the editor.
   * It should be noted that passing the `at` parameter may cause a "Cannot resolve a DOM point from Slate point" error. 
   */

  resetNodes<T extends Node>(
    editor: Editor,
    options: {
      nodes?: Node | Node[],
      at?: Location
    } = {}
  ): void {
    const children = [...editor.children]
    for (let i = 0; i < children.length; i++) {
      const node = children[i]
      editor.apply({ type: 'remove_node', path: [0], node })
    }

    if (options.nodes) {
      const nodes = Node.isNode(options.nodes) ? [options.nodes] : options.nodes
      for (let i = 0; i < nodes.length; i++) {
        editor.apply({ type: 'insert_node', path: [i], node: nodes[i] })
      }
    }

    const point = options.at && Point.isPoint(options.at) 
      ? options.at 
      : Editor.end(editor, [])
    if (point) Transforms.select(editor, point)
  }

  ...
}

Also, I believe that renaming value to initialValue could help prevent confusion around how to use slate.

What are your thoughts ?

@dylans TL;DR I think that slate should be bumped to version v1.0 on the next breaking change. Note that this PR could have been a good candidate.

I understand that slate is relatively new and is still prone to changes. Having said this, I do believe it has reached a certain level of maturity / stability - I've been using slate for over a year now, and I don't recall facing any breaking changes up until version @slate-react-v0.67.0.

My recommendation is to try and define what does stable mean for for slate. I personally think that if the majority of recent version updates are non-breaking then it is stable. Going through the changelog I feel that most of the recent updates are actually either fixes, improvements of feature addition rather than breaking changes.

Also, I don't think that a library should reach its full maturity / potential before getting bumped to a v1.0.0 release. Most libraries are continuously worked on and having a high major version number is never an issue. react is a great example of that whereby they are now working on version v18.0 and I'm sure that their first couple of versions had a lot of core things to improve and make more flexible.

Lastly, on a personal note, I've been working on a project for about 2 years now. I used to be quite protective of the version number and stayed on v0.x for a very long time. Bumping to v1 was actually very liberating: I no longer had any constraints towards version numbers and could freely use the full spectrum of semantic versioning. The project still has a lot of room for improvement and is far from stable but it is stable enough.

I also think the documentation could need some update regarding how to update the editors value.

I had a rather common use-case: async loading contents from my backend. Therefore couldnt set it as init value in useState. (docs only talk about synchronously loading from a local storage database.)
I spent quite some frustrating time playing around, reading code and docs and eventually stumbling over this discussion.

Docs left the impression to me that you can use value={content} together with useState to update the editors contents any time.

@philicious agreed about the docs, although for async loading you can simply defer rendering the editor until the content is fetched.

@philicious agreed about the docs, although for async loading you can simply defer rendering the editor until the content is fetched.

Works for the first load of the editor, but what if you want to return to the original state of the editor after changing the values?

For example:

• The UI has a form with an Edit button in the end.
• If the user clicks the Edit button, the UI hides it and then shows a Cancel and Save button.
• The initial value is loaded from a database and then the Slate component is rendered in the first time the page mounts, as suggested above.
• The user then changes the value of the slate textarea.
• Then the user press cancel.
• The expected behavior was to use the initial value that came from the database but it never happens because we lost it and we couldn't change the state externally... again.

How do you tackle this problem? Is it still with editor.children?

EDIT: I'm failing to see this at the documentation even months after. Am I missing something?

@bsides I would (try to) save the original content in state, then use editor.children = originalContent in Cancel button click handler

@bsides I would (try to) save the original content in state, then use editor.children = originalContent in Cancel button click handler

Since the original content is external, that's saving props as state, which is an anti pattern. I'm updating to editor.children for now but considering a better alternative.

@bsides ye in case you consider the content being updated from other places, you need to fetch it again and not use state. or use state + employ mutex locking in your content DB to only allow single edit instance

The thing is, this is a pretty trivial thing for a form to do: to be trully controllable in case we have external data being pulled/pushed asynchronously. Right now it's really not a good implementation because it's trying to do 2 things and failing in both.

Would like to +1 on the need to making this clearer in the documentation. For me anyway this is a very common use-case and directly setting editor.children is relatively unintuitive given the design patterns used in other React inputs. Not at all the end of the world, but an explicit note and example in the documentation could save a lot of headaches.

Is there a chance that this can be reverted? I think it was never justified in the first place, but seeing the problems it created just confirms this.

First of all, I want to say thanks to @ianstormtaylor and @bryanph for maintaining and continuing to build a great library.

Regarding these changes, I wonder whether they actually fix the issue mentioned in #3575: slate used to offer an API similar to any generic input, whereby it exposed props such as value and onChange. This enabled users to interact and maintain the value of the input with a useState hook, as below:

const Editor = () => {
  const [value, setValue] = useState('')
  return (
    <input value={value} onChange={(e) => setValue(e.target.value)} />
  )
}

Consequently, these changes entailed that to be able to interact with the value outside of slate the code would need to be changed from this:

const Editor = () => {
  const editor = useMemo(() => withReact(createEditor())), [])
  const [value, setValue] = useState('')
  return (
    <Slate
       editor={editor}
       value={value}
       onChange={setValue}
    >
      ...
    </Slate>
  )
}

to this:

const Editor = () => {
  const editor = useMemo(() => withReact(createEditor())), [])
  const [value, setValue] = useState('')
  editor.children = value
  return (
    <Slate
       editor={editor}
       value={value}
       onChange={setValue}
    >
      ...
    </Slate>
  )
}

Note the need to manually set the value of editor.children, which if I'm not mistaken can cause a Cannot resolve a DOM point from Slate point. error.

On a separate note, I think changes to the API should be added to the changelog as "Breaking Changes" instead of "Minor Changes" specially if a previous behaviour is no longer supported. I personally find it helpful to use standardised versioning system like Semantic Versioning and Keep a Changelog.

Thanks!

I am having issues with this solution now that I updated to 0.9 due to trying to fix another bug.

This solution causes a lot of side effects on 0.9 even when it used to work previously.

For example when I use Transform.setNodes now it won't update because it will trigger render with the old value, however with keypresses it will register, and only sometimes.

I think now editor.children have to be manually set every time, so while this solution may work some of the time it will not work all of the time, not anymore.

Just dropping a note for anyone using slate-history… (slate@0.94.1, slate-history@0.94.0)

Setting editor.children doesn’t save the change to your history, e.g., when your editor is this:

const editor = useMemo(() => withHistory(withReact(createEditor())), []);

I see in @KarimNahas example #4540 (comment) that we can instead use editor.apply to make changes that will be remembered.

I ran into an additional problem getting the cursor focus to remain on undo though. I tried adding Transforms.select and also Transforms.setSelection and I was seeing the selectionBefore value in editor.history.undos[0], but when I did undo, the cursor focus would get lost. As a workaround, instead of the node operations, I used "remove_text" and "insert_text" operations and that seems to work (also, I’m using Element.isElement to parse editor.children safely).

Hey there Slate community, just want to share my recent attempt at implementing this feature. Where changing the value prop updates the content shown on the editor.

const resetEditor = <T extends Node = Node>(editor: Editor, nodes?: T | T[]) => {
  const { selection } = editor;

  editor.removeNodes({ at: { anchor: editor.start([]), focus: editor.end([]) }});

  if(nodes) editor.insertNodes(Node.isNode(nodes) ? [nodes] : nodes);

  editor.select(selection ?? editor.end([]));
};

Rough example of how you could use it:

const Editor = ({ value, ...props }) => {
  const editor = useMemo(() => createEditor(), []);

  useEffect(() => {
  // Extra logic...

    resetEditor(editor, value); // Where the magic happens!
  }, [value]);

// Rest of the editor code.

Where value is the prop you pass with the slate content that you wish to be controlled.

Hope this helps ✌️!